Skip to content

std.Build.Step.Run: Set WINEDEBUG=-all for -fwine by default. #24151

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 11, 2025

Conversation

alexrp
Copy link
Member

@alexrp alexrp commented Jun 11, 2025

This silences the excessive default stderr logging from Wine. Unless the user happens to be a Wine contributor/maintainer, these messages are not helpful - rather the opposite because they drown out actually important output.

The user can still override this by setting WINEDEBUG in the environment or on the step's env_map; this just provides a more sensible default, which I think makes sense when we're already going out of our way to provide a convenient integration.

Closes #24139.

This silences the excessive default stderr logging from Wine. The user can still
override this by setting WINEDEBUG in the environment; this just provides a more
sensible default.

Closes ziglang#24139.
@alexrp alexrp requested a review from andrewrk June 11, 2025 21:44
@alexrp
Copy link
Member Author

alexrp commented Jul 11, 2025

When discussing this in the last team meeting, Andrew wasn't a huge fan of mucking with environment variables to achieve this, but left the decision to me. Adding to the PR description, I'm going to merge it because:

  • Wine's default WINEDEBUG setting is fine for end users running GUI applications, but is terrible for people running Zig applications through zig build because it spams their console output with useless messages.
    • (Unless the zig build user happens to be a Wine developer who's specifically interested in actually addressing the messages, which seems highly unlikely.)
    • This is doubly bad because the red stderr warnings demonstrably mislead people into thinking that their build has failed.
  • It's extremely unlikely that the Wine project would take a patch to change this established default because it's been this way for so many years (decades?), and it provides useful output for Wine developers to diagnose problems when end users run Windows GUI applications.
  • There is no way to achieve the effect of WINEDEBUG=-all with command line flags that I know of (unfortunately).
  • It's also unlikely that Wine would take a patch to add a command line flag; compare the rather short output of wine --help with the amount of environment variables in man wine. I think there's a clear design philosophy here.

I feel confident that this patch is the right call for most (if not all) Zig users using zig build -fwine. But we can always revert if it turns out that there's an angle I've failed to consider.

@alexrp alexrp merged commit 681d324 into ziglang:master Jul 11, 2025
10 checks passed
@alexrp alexrp deleted the wine-logging branch July 11, 2025 22:05
Comment on lines +1071 to +1072
env_map = arena.create(EnvMap) catch @panic("OOM");
env_map.hash_map = try env_map.hash_map.cloneWithAllocator(arena);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this cloning an undefined hash_map since env_map was just overwritten with the create return?

(found by https://discord.com/channels/605571803288698900/1396169875013242991/1396169875013242991)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for sharing this!

On the issue tracker, let's please avoid linking to private communities that require an account to view the content.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like an in-between of an earlier version of the patch and what I actually intended to merge. I'm not sure what went wrong here, but something did.

Thanks for the heads up. I'll put up a fix soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wine logging affects zig build test when enable_wine = true
3 participants